Create system roles by default #17341
Conversation
|
@hishamco I know this is a draft. But I think you are missing the ask. Not all these roles are system role. There are currently only 3 system roles. Administration (this name could be changed), Anonymous, and Authorized. You should inject ISystemRolePrpvider to get a list of the system roles. In the roles module, create a SystemRolesMigrations file that would read the system roles from the provider and create them using the RoleManager. Then you can remove only these 3 roles from the recipes. |
|
Thanks it is still a draft, I added the system roles first, then added the rest accidentally Using the provider is much better, I will react to your feedback |
|
@MikeAlhayek I didn't see a description there, or shall we make |
|
I tried the PR on a fresh installation and it works fine but I don't know why it fails in Linux |
src/OrchardCore.Modules/OrchardCore.Roles/Migrations/SystemRolesMigrations.cs
Show resolved
Hide resolved
| @@ -33,7 +33,10 @@ public Startup(IShellConfiguration shellConfiguration) | |||
| public override void ConfigureServices(IServiceCollection services) | |||
| { | |||
| services.AddScoped<IUserClaimsProvider, RoleClaimsProvider>(); | |||
| services.AddDataMigration<RolesMigrations>(); | |||
|
|
|||
| services.AddDataMigration<SystemRolesMigrations>() | |||
There was a problem hiding this comment.
These don't need to be groped since they don't depend on each other
There was a problem hiding this comment.
It will make no difference
There was a problem hiding this comment.
It does not. But it imply that they depend on each other
src/OrchardCore.Modules/OrchardCore.Roles/Migrations/SystemRolesMigrations.cs
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
The SaaS recipe still has these. pages.recipe.json too but I'm not sure how that's used.
There was a problem hiding this comment.
Me too :) but seems used the module for test project to test Assembly attributes if I recall
| { | ||
| if (await _roleManager.FindByNameAsync(role) is null) | ||
| { | ||
| await _roleManager.CreateAsync(new Role { RoleName = role }); |
There was a problem hiding this comment.
The descriptions of the roles need to be retained too.
There was a problem hiding this comment.
I already pointed to that in #17341 (comment)
Fixes #17271